Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Introduce AssignExpression to support multiple assignments #1105

Merged
merged 1 commit into from
Mar 9, 2023
Merged

Introduce AssignExpression to support multiple assignments #1105

merged 1 commit into from
Mar 9, 2023

Conversation

oxisto
Copy link
Member

@oxisto oxisto commented Feb 23, 2023 •
edited
Loading

This adds a new statement class, called AssignExpression as well as TupleType.

The AssignExpression holds a list of lhs and rhs for the respective values on the left and right side. Furthermore, it contains a list of declarations, since some languages can declare variables in an assignment-like statement (either implicitly or explicitly). The recommended way to fill these declarations are in an extra pass that runs after all language frontends have been executed.

@oxisto oxisto linked an issue Feb 24, 2023 that may be closed by this pull request
Support multiple return values #1089
Open
@oxisto oxisto force-pushed the multiple-return-types branch from 7cc4405 to d1dc827 Compare February 24, 2023 09:15
@oxisto oxisto mentioned this pull request Feb 24, 2023
Open
@oxisto oxisto removed a link to an issue Feb 24, 2023
Support multiple return values #1089
Open
@oxisto oxisto changed the title First experiment to model multiple variable assignments Introduce AssignStatement Feb 24, 2023
@oxisto oxisto changed the title Introduce AssignStatement Introduce AssignStatement to support multiple assignments Feb 24, 2023
@oxisto oxisto force-pushed the multiple-return-types branch from d1dc827 to 764efd2 Compare February 24, 2023 10:21
@oxisto oxisto marked this pull request as ready for review February 27, 2023 15:54
@oxisto oxisto changed the title Introduce AssignStatement to support multiple assignments Introduce AssignExpression to support multiple assignments Feb 27, 2023
@oxisto oxisto changed the title Introduce AssignExpression to support multiple assignments Introduce AssignExpression to support multiple assignments Feb 27, 2023
@oxisto oxisto force-pushed the multiple-return-types branch 4 times, most recently from 9c63b9c to f0851ce Compare March 4, 2023 12:58
@oxisto oxisto mentioned this pull request Mar 4, 2023
Merged
@oxisto oxisto force-pushed the multiple-return-types branch 2 times, most recently from 7c30414 to 83f3b65 Compare March 7, 2023 09:24
KuechA
KuechA reviewed Mar 7, 2023
View reviewed changes
Copy link
Contributor

@KuechA KuechA left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised that there's no change necessary to the ControlFlowSensitiveDFGPass.

cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/declarations/VariableDeclaration.kt Outdated Show resolved Hide resolved
...ore/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/statements/expressions/AssignExpression.kt Outdated Show resolved Hide resolved
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/types/TupleType.kt Show resolved Hide resolved
@oxisto oxisto force-pushed the multiple-return-types branch from 99aac98 to 3e59d28 Compare March 8, 2023 10:43
@oxisto oxisto mentioned this pull request Mar 8, 2023
Closed
@oxisto oxisto force-pushed the multiple-return-types branch 2 times, most recently from 9c54b66 to a3b5c69 Compare March 8, 2023 11:09
@oxisto
First experiment to model multiple variable assignments
9ba037f 
This adds a new statement class, called `AssignStatement` as well as `TupleType`.
@oxisto oxisto force-pushed the multiple-return-types branch from a3b5c69 to 9ba037f Compare March 8, 2023 11:40
@sonarcloud
Copy link

sonarcloud bot commented Mar 8, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

53.8% 53.8% Coverage
0.0% 0.0% Duplication

@oxisto
Copy link
Member Author

oxisto commented Mar 8, 2023

I'm surprised that there's no change necessary to the ControlFlowSensitiveDFGPass.

To be honest, I have just not touched that yet, because for me this pass is black voodoo magic. There are some checks in the pass for = assignments in binary operators. Those probably need to be adjusted to the AssignExpression.

@KuechA
Copy link
Contributor

KuechA commented Mar 8, 2023

I'm surprised that there's no change necessary to the ControlFlowSensitiveDFGPass.

To be honest, I have just not touched that yet, because for me this pass is black voodoo magic. There are some checks in the pass for = assignments in binary operators. Those probably need to be adjusted to the AssignExpression.

Ok, I guess I should look at it but we can do that in a separate PR. I just realized that we should probably also change the assign functions (or offer a "BinaryOperator" and a "AssignmentExpression" one) in the Fluent API and offer a tuple type there as well, right?

@oxisto
Copy link
Member Author

oxisto commented Mar 8, 2023

I'm surprised that there's no change necessary to the ControlFlowSensitiveDFGPass.

To be honest, I have just not touched that yet, because for me this pass is black voodoo magic. There are some checks in the pass for = assignments in binary operators. Those probably need to be adjusted to the AssignExpression.

Ok, I guess I should look at it but we can do that in a separate PR. I just realized that we should probably also change the assign functions (or offer a "BinaryOperator" and a "AssignmentExpression" one) in the Fluent API and offer a tuple type there as well, right?

Yes that would be good; I would only offer the AssignExpression one. We want to get rid of the binary op assignment at some point.

@oxisto oxisto requested a review from konradweiss March 8, 2023 18:09
@oxisto oxisto merged commit fee415e into main Mar 9, 2023
@oxisto oxisto deleted the multiple-return-types branch March 9, 2023 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KuechA KuechA KuechA left review comments

@maximiliankaul maximiliankaul maximiliankaul left review comments

@konradweiss konradweiss konradweiss approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@oxisto @KuechA @konradweiss @maximiliankaul